Skip to content

feat: serve markdown to agents, and apply the _headers file sites ship - #23

Merged
aryanmehrotra merged 8 commits into
mainfrom
feat/markdown-content-negotiation
Aug 4, 2026
Merged

feat: serve markdown to agents, and apply the _headers file sites ship#23
aryanmehrotra merged 8 commits into
mainfrom
feat/markdown-content-negotiation

Conversation

@aryanmehrotra

@aryanmehrotra aryanmehrotra commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Two independent fixes to what this server sends, plus a config-resolution bug found by running the real image. All additive and gated on something the published directory opts into, so a deployment that does neither is byte-for-byte unchanged. Reviewable commit by commit.


1. Serve markdown to agents (Accept: text/markdown)

Static site generators already emit about.md next to about/index.html. The file is on disk — the server just never looked for it. No build changes needed in any consuming site.

Without this, an agent can only discover the markdown by reading the <link rel="alternate"> inside the HTML document it was trying not to download. It pays full price first, which defeats the point.

Measured on a real build of zop.dev, which this server hosts:

Page HTML Markdown Ratio
/changelog/v1-33-0 145,354 B 1,512 B 96×
/docs/zopnight/introduction 228,960 B 3,866 B 59×
/learn/1-hop-adjacency 170,523 B 9,525 B 18×
/resources/blogs/agentic-ai-finops… 172,719 B 11,334 B 15×

Per Checkly's Feb 2026 survey, Claude Code, Cursor and OpenCode send this header today. On zop.dev, AI agents are ~30% of external requests — roughly 1:1 with human browsers.

Misses are answered in markdown too. An HTML error shell is unusable to such a client and not small: a real site's 404 page measured 144,188 bytes, sent in reply to a request the client could not parse. The markdown reply is 138 bytes and names /sitemap.xml and /llms.txt so the reader can recover. The request path is deliberately not echoed into it — gosec flagged that as an injection sink (G705).

Safety

  • Only explicit media types count. A browser sends */*;q=0.8, which matches text/markdown by the letter of RFC 9110. Matching wildcards would serve source to every visitor on the internet. Pinned by tests using verbatim Chrome and Safari Accept strings.
  • q-values are honoredtext/html, text/markdown;q=0.1 still gets HTML.
  • Falls through untouched when the client didn't ask, the .md is absent, or the URL has an extension.
  • Vary: Accept, scoped to what can actually vary — without it a CDN can hand an agent's markdown to the next browser. It is set on the extensionless routes that can resolve to a .md, and on every miss (a miss answers in markdown whenever asked, whatever the path looks like). It is not set on assets: a hashed bundle can never negotiate, so keying caches on Accept there would fragment them for nothing — on precisely the responses _headers marks immutable.
  • Content-Type set explicitly, for negotiated responses only. Go's MIME table has no .md entry and distroless has no /etc/mime.types, so ServeFile would sniff text/plain. A directly requested .md is left alone: browsers render text/plain inline but download text/markdown, so relabeling it would turn a site's existing .md links into download prompts. Worth knowing this differs by platform — most Linux distributions map .md, so a dev box already serves text/markdown while the distroless image that ships does not. The decision is unit-tested rather than asserted through a served header, which would only pin the host's MIME table.

2. Apply the _headers file the published directory ships

The Netlify / Cloudflare Pages convention. Generators emit it expecting the host to honor it; a host that ignores it fails silently while the file looks authoritative in the repo.

zop.dev has shipped a 2,437-byte _headers for months. Measured against the live site, not one rule was in effect:

Declared Live
X-Frame-Options: DENY absent
X-Content-Type-Options: nosniff absent
Referrer-Policy absent
Permissions-Policy absent
Cache-Control on /_astro/* (immutable) absent entirely

Content-hashed bundles that could be cached for a year were served with no caching directive at all.

Parsed once at startup. No file → no rules → unchanged, so a published directory that ships none behaves exactly as it does today. All matching rules contribute in file order so a later block overrides an earlier catch-all. Malformed lines are skipped rather than failing the file. Applied before anything writes, so they cover hits, misses and the SPA fallback — a 404 that leaks framing protection is as exploitable as a 200 that does.

With one exception: Cache-Control is dropped on a miss. A site's cache directives describe the pages it publishes, not the ones it doesn't have. A /*.html block with max-age=300 would otherwise pin a file that is merely un-propagated mid-deploy into every cache downstream for the rule's full lifetime. The SPA fallback keeps its caching — that is a real route being served, not a miss.

Delegated paths count as the site too. /.well-known/ is handed to the next handler untouched, so an ACME challenge is never given an extension or swallowed by the SPA fallback. That early return also skipped the header rules, so a site declaring X-Frame-Options for /* got it everywhere except there. Rules are now applied before the branch — they depend only on the request path — and delegation itself is unchanged, verified by a test that pins the path arriving unrewritten. Since the delegate picks its own status, the cache directives are withdrawn on the way out rather than up front:

Delegated outcome Security headers Cache-Control
200 — a real ACME/.well-known file applied kept
404 — absent applied withdrawn

The response writer is wrapped only on that path: doing it on the main serving path would hide net/http's io.ReaderFrom from http.ServeFile and cost every static file its sendfile fast path.

Operator note: a reverse proxy in front of this server may set some of these itself. Strict-Transport-Security is the usual one (ingress-nginx sends max-age=15724800, no preload, with replace semantics) — where that's the case it will most likely keep winning, so a preload declared in this file is unlikely to suddenly go live. The other five rarely have a proxy counterpart.


3. Empty config values fell back to nothing

Found by running the actual distroless image rather than a native binary.

GetOrDefault only falls back when a key is absent. The shipped configs/.env sets STATIC_DIR_PATH= and DEFAULT_EXTENSION= empty, so a deployment supplying STATIC_DIR_PATH via the environment got "" — every lookup silently rooted at the process working directory:

Container shape before after
default ./static 22 rules ✅ 22 ✅
config-file STATIC_DIR_PATH 22 rules ✅ 22 ✅
env-var STATIC_DIR_PATH 0 rules, pages still served 22 ✅

The default shape was unaffected — but by luck, not design. The startup line now names the resolved directory, since "0 rules" is normal for a site without the file and indistinguishable from a misrooted path otherwise.


Testing

  • go test -race green. 20 Accept-parsing cases; _headers suite built on a verbatim excerpt of the real zop.dev file.
  • Mutation-tested — the suite goes red on each of: treating */* as markdown; ignoring q-values; negotiating with no .md; reverting the markdown 404; not applying header rules; applying only the first matching rule; dropping pattern anchoring; Vary unconditional on hits; Vary dropped on a miss; the Cache-Control withdrawal removed; the markdown Content-Type unscoped; negotiable() ignoring either the extension or the root; .well-known skipping the rules; the delegated writer unwrapped; scrubbing on every status instead of errors only; withdrawCacheDirectives as a no-op.
  • Run under golang:1.26 with media-types installed, so /etc/mime.types really contained text/markdown md markdown — the platform shape that behaves differently from a dev Mac and from the distroless image.
  • golangci-lint: 1 finding, against 4 on origin/main — nothing new. The three that dropped are gosec G703 taint hits on http.ServeFile, whose call sites are untouched; gosec's taint walk is sensitive to unrelated edits in the same function.

Verified in the real image

gcr.io/distroless/static-debian12, built from this Dockerfile, serving a real 5,978-page build — 16/16: correct Content-Type for html/css/svg/txt/md with no /etc/mime.types; every _headers rule applied including on 404s; markdown negotiation; Vary; 138-byte markdown 404; pages without a .md still 200.

Regression evidence

Base and patched binaries run side by side on the same build, diffing status, Content-Type, Vary, Cache-Control, X-Frame-Options, Content-Length and body SHA-256 across path × Accept combinations — a browser string, */*, text/html, text/markdown;q=0.1, text/markdown, and no header.

Across the whole matrix the only changed-or-removed field is the one intended negotiation (/about + Accept: text/markdown). Every other delta is an addition of a header the site's own _headers file declares:

Path Delta vs main
/, /index.html, /style.css, /robots.txt, /readme.md + Cache-Control, + X-Frame-Options
/_astro/app.*.js + Cache-Control: …immutable, + X-Frame-Optionsno Vary
/about, /about/ + Cache-Control, + X-Frame-Options, + Vary: Accept
/missing + X-Frame-Options, + Vary: Acceptno Cache-Control
/.well-known/acme-challenge/<tok> + Cache-Control, + X-Frame-Options — on main this response carried no headers at all; body unchanged
/about + text/markdown the intended markdown body, + Vary, + Content-Type: text/markdown

/readme.md no longer appears as a Content-Type change; it is byte- and header-identical to main apart from the _headers additions.

An earlier run before the _headers commit compared 1,505 pairs across 5 non-markdown Accept variants with 0 mismatches — though note that run compared status + Content-Type + body only, which is why the Vary and Cache-Control scoping above needed the wider diff to surface.

Rollout

The markdown half is inert until the site being served actually emits .md siblings — a build that doesn't is byte-for-byte unchanged, so this can land ahead of any generator work. The _headers and config halves are independent and take effect as soon as a directory ships a _headers file.

Static site generators emit the markdown source of a page as a sibling of
its directory index — `about.md` next to `about/index.html`. The file is
already on disk; the server just never looked for it.

Now a request whose Accept header names text/markdown is served that
sibling. Without this an agent can only discover the markdown by reading
the <link rel="alternate"> inside the HTML document it was trying not to
download. Measured on a real zop.dev build, the same page is 15-96x
smaller as markdown (a changelog entry: 145,354 bytes of HTML vs 1,512).

Only explicit media types count. A browser sends `*/*;q=0.8`, which
matches text/markdown by the letter of RFC 9110, so matching wildcards
would serve raw source to every human visitor; q-values are honoured, so
a client that ranks markdown below HTML still gets HTML. The lookup falls
through untouched when the client did not ask or the .md is absent, so
nothing an existing deployment serves today changes.

Vary: Accept is set on every response — without it a CDN can hand an
agent's markdown to the next browser that asks for the same URL.

.md is registered with the mime package explicitly: Go's built-in table
has no entry for it and a scratch base image has no /etc/mime.types, so
http.ServeFile would otherwise sniff the file and label it text/plain.

Verified end to end against a 5,933-page build: agents get markdown,
browsers get HTML, pages without a .md still return 200, and direct .md
requests are unaffected. Handler and negotiation are at 100% statement
coverage.
Lint fixes for the CI gate:
  - drop the init() (gochecknoinits) — the .md content type is now set
    explicitly in the handler, which also covers directly-requested .md
    rather than relying on process-global MIME registration;
  - split Accept parsing into parseAcceptEntry so markdownPreferred drops
    back under the cyclomatic limit;
  - "honoured" -> "honored" (misspell, US locale).

Also answers a 404 in markdown when the client asked for markdown. An
HTML error shell is unusable to such a client and is not small: the 404
page of a real site measured 144,188 bytes, sent in reply to a request
it could not parse. The markdown reply is 138 bytes and names
/sitemap.xml and /llms.txt so the reader can recover on its own.

The request path is deliberately not echoed into that body — gosec
flagged it as an injection sink (G705), and the caller already knows the
URL it asked for.

Verified no behaviour change for anyone else: base and patched binaries
served the same 5,933-page build and were compared over 1,505
request/response pairs (301 URLs x 5 non-markdown Accept variants,
including a browser string and `text/html, text/markdown;q=0.1`).
Status, Content-Type and body SHA-256 matched on every one. The only
deltas are the added Vary: Accept and markdown for clients that asked.

golangci-lint output is identical to origin/main — the diff introduces
no new findings.
@aryanmehrotra aryanmehrotra changed the title feat: serve markdown to agents via Accept content negotiation feat: serve markdown to agents, and apply the _headers file sites ship Jul 31, 2026
`_headers` is the Netlify / Cloudflare Pages convention: a file at the
root of the published directory listing path patterns and the response
headers to send for them. Static site generators emit it expecting the
host to honour it, and a host that ignores it fails in the worst way —
the file looks authoritative in the repo while nothing it declares has
ever reached a browser.

https://zop.dev, served by this project, has shipped a 2,437-byte
_headers for months. Measured against the live site, not one of its
rules was in effect:

  X-Frame-Options: DENY                    absent
  X-Content-Type-Options: nosniff          absent
  Referrer-Policy                          absent
  Permissions-Policy                       absent
  Cache-Control on /_astro/* (immutable)   absent entirely

That last one matters twice over: content-hashed bundles that could be
cached for a year were being served with no caching directive at all.

Rules are parsed once at startup. A published directory without a
_headers file yields no rules, so an existing deployment is
byte-for-byte unchanged.

All matching rules contribute in file order, so a later specific block
overrides an earlier catch-all, matching upstream precedence. Malformed
lines are skipped rather than failing the file: one bad rule should not
cost a site every other header it declares. Headers are applied before
anything writes, so they cover hits, misses and the SPA fallback alike —
a 404 that leaks framing protection is as exploitable as a 200 that does.

Note for operators: if a reverse proxy sits in front of this server, it
may set some of these itself. Strict-Transport-Security is the usual
one (ingress-nginx sends max-age=15724800, no preload, with replace
semantics), and where it does it will keep winning; the other five
headers rarely have a proxy counterpart and take effect immediately.

Verified against a real 5,933-page build: 22 rules load, and the expected
headers appear on pages, hashed assets, the root and robots.txt. Compared
891 request/response pairs against origin/main — zero body differences,
zero unintended differences. Handler and parser at 100%/96% statement
coverage; golangci-lint output identical to origin/main.
Found by running the actual distroless image rather than a native binary.

`GetOrDefault` only falls back when a key is ABSENT. The shipped
configs/.env sets STATIC_DIR_PATH= and DEFAULT_EXTENSION= with empty
values, so a deployment that supplies STATIC_DIR_PATH through the
environment got "" instead — every path lookup silently rooted at the
process working directory. In that shape the server still served pages
but loaded zero _headers rules, which is exactly the kind of half-working
state that never gets noticed.

The default shape (./static, where a Dockerfile typically copies the
published directory) was unaffected and loaded all 22 rules. This makes
the other shapes behave the same.

The startup line now names the resolved directory alongside the count.
"0 rules" is normal for a site with no _headers file and indistinguishable
from a misrooted path unless the path is on the line too.

Verified in the real gcr.io/distroless/static-debian12 image against a
5,978-page build, 16/16: correct Content-Type for html/css/svg/txt/md
with no /etc/mime.types present, all _headers rules applied including on
404s, markdown negotiation, Vary, and a 138-byte markdown 404.
@aryanmehrotra
aryanmehrotra force-pushed the feat/markdown-content-negotiation branch from 37040a2 to 7444624 Compare August 3, 2026 06:38
…long

Three header changes reached responses that cannot benefit from them. Each was
found by diffing the real binaries against origin/main rather than by reading
the diff — the earlier comparison covered status, Content-Type and body only,
so nothing had ever checked Vary or Cache-Control.

Vary: Accept was sent on every response, including the hashed bundles under
/_astro/ that the same _headers file marks immutable. Only an extensionless
route can resolve to a .md sibling, so everything else was keying caches on a
header that cannot change what they return — the cost landing precisely on the
responses this server most wants cached. It is now gated on the same condition
resolveFilePath uses to negotiate, and negotiable() is the one definition of
that condition so the two cannot drift apart.

A miss still varies whatever the path looks like, because a miss is answered in
markdown whenever the client asked for it — including for extensions that never
negotiate on a hit. Without that a cache can hand an agent the HTML shell it
stored for a browser.

A site's Cache-Control reached its 404s. `/*.html` with max-age=300 meant a
file merely not propagated yet during a deploy was pinned into every cache
downstream for the rule's lifetime. Cache directives are now dropped on a miss.
The security headers still apply — a 404 that leaks framing protection is as
exploitable as a 200 that does — and the SPA fallback keeps its caching, since
that is a real route being served, not a miss.

Directly requested .md files were relabelled text/plain -> text/markdown.
Bodies were identical, which is how it read as a no-op, but browsers render
text/plain inline and download text/markdown: every existing .md link on a site
would have turned into a download prompt. The explicit type is now set only for
a negotiated response, which is the case that needs it — Go's MIME table has no
.md entry and distroless has no /etc/mime.types.

Verified against base and patched binaries on the same build across 13
path x Accept combinations, comparing status, Content-Type, Vary, Cache-Control,
X-Frame-Options, Content-Length and body SHA-256. The only changed or removed
field in the whole matrix is the one intended negotiation; every other delta is
a header the site's own _headers file declares. Six mutations go red: Vary
unconditional on hits, no Vary on a miss, the Cache-Control strip removed, the
markdown Content-Type unscoped, and negotiable() ignoring either the extension
or the root.

golangci-lint output is identical to origin/main (4 findings, all pre-existing).
CI caught what a macOS run could not. The previous commit asserted that a
directly requested .md keeps a non-markdown Content-Type — true on macOS and in
the distroless image that ships, false on the Ubuntu runner, because most Linux
distributions map .md in /etc/mime.types and http.ServeFile answers text/markdown
there before this server does anything. The assertion pinned the host's MIME
table rather than any behaviour of ours, so it passed locally and failed in CI.

The scoping itself was right and is unchanged. What changes is how it is
checked: the condition moves into labelAsMarkdown, covered by a table that runs
identically everywhere, and the end-to-end test now compares a direct .md
against mime.TypeByExtension — the same lookup ServeFile makes — instead of
against a hardcoded type. Where the platform has no entry that is text/plain,
which is the production case and the one the scoping exists for; where it has
one, the test agrees with it. A negotiated response is asserted to carry the
explicit type on every platform, since that is the case that must not depend on
the base image having a MIME table at all.

Verified by running the suite under golang:1.26 with media-types installed, so
/etc/mime.types really did contain `text/markdown md markdown` — the exact shape
that failed. All six mutations still go red; golangci-lint remains identical to
origin/main.
.well-known is handed to the next handler untouched, so that an ACME challenge
is not given an extension or swallowed by the SPA fallback. That early return
also skipped the _headers rules, so a site declaring X-Frame-Options for `/*`
got it everywhere except there. A `/*` block means the whole site, and a path
this server delegates is still a path it answered for.

The rules are now applied before the branch, which is where they belonged: they
depend only on the request path, not on anything the resolution step produces.
Delegation itself is unchanged — the path reaches the next handler exactly as
before, with no rewriting.

That raises the question the miss path already answered: the site's
Cache-Control must not attach to a response that is not a page the site
publishes. Here the status is chosen by the delegate, so it cannot be decided up
front, and the directives are withdrawn on the way out instead — a delegated 200
is a real file and keeps its caching, a delegated 404 does not. Both paths now
call withdrawCacheDirectives, so the rule has one definition and one rationale
rather than two that can drift.

The writer is wrapped only for the delegated paths. Wrapping the main serving
path would hide net/http's io.ReaderFrom from http.ServeFile and cost every
static file its sendfile fast path; ACME challenges are small and rare enough
not to be worth a special case to keep fast.

Verified against the real binary with GoFr's own chain as the delegate: on main
an ACME challenge file comes back with no security headers at all, and with this
change it carries X-Frame-Options while still returning the token body intact,
while an absent .well-known path 404s with the security headers and without
Cache-Control. Four mutations go red: skipping the rules for .well-known,
dropping the wrapper, scrubbing on every status rather than errors only, and
making withdrawCacheDirectives a no-op.

golangci-lint reports fewer findings than origin/main rather than more (1 vs 4).
The three that went are gosec G703 taint-analysis hits on http.ServeFile, whose
call sites this commit does not touch — gosec's taint walk is sensitive to
unrelated edits in the same function.
@PiyushSingh-ZS

Copy link
Copy Markdown
Contributor

Review

Solid, careful work. Core logic — Accept negotiation, _headers parsing/precedence, Vary/Cache-Control scoping, .well-known delegation — is correct and well-tested. Verified locally: go build ./..., go test ./..., and golangci-lint run all pass clean on the branch. Findings below are two real gaps plus minor nits; none block.

1. [Medium — docs] README not updated for two user-facing features

The diff touches only .go files. README.md documents configuration and resolution behavior in tables, but neither new behavior is mentioned:

  • Markdown content negotiation (Accept: text/markdown.md sibling; Vary: Accept; markdown 404s) — negotiate.go, handler.go.
  • _headers file support (parsed at startup) — headers.go, main.go.

The README is the repo's sole doc surface and already carries a behavior contract. Shipping _headers support and Accept negotiation without documenting them there is the exact "file looks authoritative but nothing reads it" failure mode this PR argues against for _headers. Suggest adding short "Content negotiation" and "_headers" subsections (behavior, Vary scoping, the miss/Cache-Control rule).

2. [Medium — testing] TestEmptyConfigValuesFallBackToDefaults pins nothing

main_test.go:

got := tt.value
if got == "" {
    got = tt.fallback
}
assert.Equal(t, tt.want, got)

This re-implements the fallback inline and never calls the production code it claims to cover. Reverting the guard in main.go (if staticFilePath == "" / if defaultExtension == "") leaves this test green — it's shape-pinning, not boundary-pinning. TestServer builds the handler with an explicit staticFilePath and never runs main()'s resolution either, so the empty→default guard — the actual bug this commit fixes — has no test that would catch a regression. Notable given the PR's mutation-testing narrative; this one wouldn't go red.

Suggest extracting the empty→default resolution into a small helper (e.g. resolveOrDefault(cfg, key, def)) and table-testing that, so the assertion exercises the real code path.

3. [Low — correctness/edge] SPA fallback can serve a negotiable route without Vary: Accept

On a miss with spaMode && !hasExtension, the handler serves index.html and returns before the negotiable()Vary block. For an extensionless route that has a .md sibling but no HTML page, the agent gets foo.md with Vary: Accept (hit path) while a browser gets index.html via SPA fallback without Vary — a shared cache keyed without Vary could cross-serve. Narrow (requires SPA_MODE=true and a .md with no sibling HTML, unusual for SSG output), but a gap in the otherwise-careful Vary reasoning. Suggest setting Vary: Accept on the SPA-fallback response when negotiable(r.URL.Path), or noting why it's safe to omit.

4. [Nit] Vary added via Add, not Set

handler.go uses w.Header().Add("Vary", "Accept") on the hit and miss paths. If a site's _headers ever declares its own Vary for a matching pattern (applied via Set), the response gets Vary: <declared>, Accept — harmless per spec, but a merged value. Set/de-dup would be marginally cleaner. Not a defect.


Checked and correct: wildcard safety (*/*;q=0.8 never matches; q=0, tie-breaks, legacy text/x-markdown all handled); negotiable() as single source of truth for negotiation + Vary + Content-Type; shared withdrawCacheDirectives on miss and delegated 4xx with correct WriteHeader-before-write ordering; _headers file-order precedence, QuoteMeta anchoring, first-colon Cut, malformed-line skipping, CRLF; direct .md left untouched with a platform-honest test; no request-path reflection in the 404 markdown body.

🤖 Generated with Claude Code

…d for real

Four items from review, all confirmed against the code before acting.

The empty-config test was vacuous, and the mutation narrative made that worse
rather than better. It re-implemented the fallback in its own body:

    got := tt.value
    if got == "" { got = tt.fallback }
    assert.Equal(t, tt.want, got)

which passes by construction and never touches main.go. Deleting both guards
from main() left the suite green — the one bug that commit fixes had no test at
all. The resolution now lives in resolveOrDefault, taking the narrow config
interface it needs, and is table-tested through the real function against a fake
whose keys are present-but-empty, which is the distinction that matters and the
one configs/.env actually ships. Removing the guard now fails.

The SPA fallback could serve a negotiable route without Vary: Accept. With
foo.md on disk and no HTML page beside it, markdown clients take the hit path
and browsers land on the shell, so one URL yields two bodies while only one of
them said it varies — a shared cache could hand the shell to an agent.
Reproduced against a running server before fixing.

Vary is now advertised through one helper, which keeps Add over Set: a site's
_headers may declare its own Vary and Set would discard it, while repeated field
lines are combined by caches, so a declared `Vary: Accept-Encoding` plus ours
reads as `Accept-Encoding, Accept`. What the helper adds is idempotence, so a
site that already named Accept does not end up with `Accept, Accept`.

The README documented neither content negotiation nor _headers, though it is the
repo's only doc surface and already carries a behaviour contract. Shipping
_headers support undocumented is the same failure this PR argues against for
_headers itself. Both are now described, including the Vary scoping, the
Cache-Control-on-miss rule, `.well-known`, and the empty-value config fallback.
Every claim in those sections was checked against a running server rather than
written from memory: pattern anchoring both ways, no Vary on assets, a
site-declared Vary surviving, Cache-Control withdrawn from a miss while
X-Frame-Options is not, the legacy text/x-markdown spelling, and the startup log.

Mutation coverage extended to all of it: resolveOrDefault ignoring empty, the
SPA fallback dropping Vary or advertising it unconditionally, and the
idempotence guard removed. The unconditional case is only observable for a root
with no index.html, so that shape is pinned explicitly rather than left as an
unverified assertion. Suite green under golang:1.26 with media-types installed;
golangci-lint unchanged at 1 finding against origin/main's 4.
@PiyushSingh-ZS

Copy link
Copy Markdown
Contributor

Re-review — 75f4c8a

All four findings addressed, and the fixes are correct. Verified locally: go build ./..., go test ./..., and golangci-lint run all clean.

1. Docs (medium) — resolved. New "Content Negotiation" and "Response Headers (_headers)" sections in the README, plus a note that a set-but-empty variable falls back to its default. The behavior tables are accurate — I spot-checked the anchoring claim (/docs/*^/docs/.*$ doesn't match /other/docs/x), * spanning /, direct-.md left as-is, and the Cache-Control-on-miss rule against the code.

2. Config test (medium) — resolved, and properly boundary-pinning now. The tautology is gone: resolution moved into resolveOrDefault(cfg configLookup, key, fallback) (main.go), and TestResolveOrDefault (main_test.go) drives the real function through a fakeConfig that distinguishes present-but-empty from absent — the exact distinction configs/.env trips. I confirmed by mutation: reverting the guard to a bare GetOrDefault turns the suite red (present but empty … falls back fails). Nice touch keeping the whitespace is kept case so the fix doesn't over-reach.

3. SPA-fallback Vary (low/edge) — resolved. negotiable() now gates advertiseAcceptVaries on the SPA-fallback branch (handler.go), and TestSPAFallbackAdvertisesVary pins both halves of the pair (browser→shell with Vary, agent→.md). The extra TestSPAFallbackRootIsNeverKeyed correctly pins that the root — reached only when index.html is absent — stays unkeyed.

4. Vary Add-vs-Set (nit) — resolved better than suggested. Rather than Set (which I'd noted would clobber a site-declared Vary), advertiseAcceptVaries (negotiate.go) keeps Add and adds idempotence: it scans existing Vary field lines case-insensitively and skips if Accept is already named, so a declared Vary: Accept-Encoding correctly becomes Accept-Encoding, Accept while an existing Accept doesn't double. TestAdvertiseAcceptVaries covers the list and case-fold cases.

LGTM. 👍

@PiyushSingh-ZS PiyushSingh-ZS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit-by-commit; all four earlier review findings addressed correctly in 75f4c8a. Verified locally: build, go test ./..., and golangci-lint all clean, and the config-guard fix is mutation-tested (goes red when reverted). LGTM.

Note: CI is currently red on the head commit (both jobs failed in ~2–3s, which reads as a setup/runner failure rather than a code failure given the suite passes locally) — worth a re-run before merge.

@aryanmehrotra
aryanmehrotra merged commit c0f09e5 into main Aug 4, 2026
6 of 8 checks passed
@aryanmehrotra
aryanmehrotra deleted the feat/markdown-content-negotiation branch August 4, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants